GH-46004: [C++][FlightRPC] Enable ODBC Build In C++ Workflows #47689

alinaliBQ · 2025-10-02T21:14:30Z

Rationale for this change

Enable ODBC Build In C++ Workflows to make sure ODBC can build in CI.

What changes are included in this PR?

Make environment variable ARROW_FLIGHT_SQL_ODBC:ON recognized in C++ build.
Register ODBC driver dll

Are these changes tested?

Tested in CI

Are there any user-facing changes?

No

Extracted from PR #46099

GitHub Issue: [C++] Add ODBC build option to arrow root cmake file #46004

alinaliBQ · 2025-10-02T21:15:55Z

cpp/src/arrow/flight/sql/odbc/flight_sql/utils.h

 #include <arrow/flight/types.h>
+#include <boost/variant.hpp>


boost::variant is an existing dependency of flightsql-odbc. Our plan is to replace it with std::variant in future PRs

alinaliBQ · 2025-10-08T16:19:49Z

.github/workflows/ruby.yml

@@ -219,6 +219,7 @@ jobs:
      ARROW_DATASET: ON
      ARROW_FLIGHT: ON
      ARROW_FLIGHT_SQL: ON
+      ARROW_FLIGHT_SQL_ODBC: ON


ARROW_FLIGHT_SQL_ODBC: ON is already on cpp.yml file

https://github.com/apache/arrow/blob/66b2f4af147bf2af3077b3addf49fcf5b8629d52/.github/workflows/cpp.yml#L400

alinaliBQ · 2025-10-09T20:22:39Z

@lidavidm @kou The PR for building ODBC is ready for review. Please have a look, thanks

kou · 2025-10-10T00:35:33Z

cpp/src/arrow/flight/sql/odbc/odbc_impl/ui/custom_window.cc

  switch (msg) {
    case WM_NCCREATE: {
-      _ASSERT(lparam != NULL);
+      assert(lparam != NULL);


Can we use ARROW_DCHECK*() instead?

arrow/cpp/src/arrow/util/logging.h

Lines 90 to 125 in 5750e29

# ifdef NDEBUG

# define ARROW_DFATAL ::arrow::util::ArrowLogLevel::ARROW_WARNING

// CAUTION: DCHECK_OK() always evaluates its argument, but other DCHECK*() macros

// only do so in debug mode.

# define ARROW_DCHECK(condition) \

while (false) ARROW_IGNORE_EXPR(condition); \

while (false) ::arrow::util::detail::NullLog()

# define ARROW_DCHECK_OK(s) \

ARROW_IGNORE_EXPR(s); \

while (false) ::arrow::util::detail::NullLog()

# define ARROW_DCHECK_EQ(val1, val2) \

while (false) ARROW_IGNORE_EXPR(val1); \

while (false) ARROW_IGNORE_EXPR(val2); \

while (false) ::arrow::util::detail::NullLog()

# define ARROW_DCHECK_NE(val1, val2) \

while (false) ARROW_IGNORE_EXPR(val1); \

while (false) ARROW_IGNORE_EXPR(val2); \

while (false) ::arrow::util::detail::NullLog()

# define ARROW_DCHECK_LE(val1, val2) \

while (false) ARROW_IGNORE_EXPR(val1); \

while (false) ARROW_IGNORE_EXPR(val2); \

while (false) ::arrow::util::detail::NullLog()

# define ARROW_DCHECK_LT(val1, val2) \

while (false) ARROW_IGNORE_EXPR(val1); \

while (false) ARROW_IGNORE_EXPR(val2); \

while (false) ::arrow::util::detail::NullLog()

# define ARROW_DCHECK_GE(val1, val2) \

while (false) ARROW_IGNORE_EXPR(val1); \

while (false) ARROW_IGNORE_EXPR(val2); \

while (false) ::arrow::util::detail::NullLog()

# define ARROW_DCHECK_GT(val1, val2) \

while (false) ARROW_IGNORE_EXPR(val1); \

while (false) ARROW_IGNORE_EXPR(val2); \

while (false) ::arrow::util::detail::NullLog()

Sure, I have changed to use ARROW_DCHECK here.

kou · 2025-10-10T00:35:57Z

cpp/src/arrow/flight/sql/odbc/install/install_amd64.cmd

+	reg add "HKEY_LOCAL_MACHINE\SOFTWARE\ODBC\ODBCINST.INI\Apache Arrow Flight SQL ODBC Driver" /v Setup /t REG_SZ /d %ODBC_AMD64% /f
+	reg add "HKEY_LOCAL_MACHINE\SOFTWARE\ODBC\ODBCINST.INI\ODBC Drivers" /v "Apache Arrow Flight SQL ODBC Driver" /t REG_SZ /d "Installed" /f
+
+	IF !ERRORLEVEL! NEQ 0 (


Suggested change

IF !ERRORLEVEL! NEQ 0 (

if !ERRORLEVEL! NEQ 0 (

BTW, is !ERRORLEVEL! still non 0 when the first reg add succeeded and the last reg add failed?

!ERRORLEVEL! should be reflecting on the last command executed, so yes !ERRORLEVEL! should be non zero if last reg add failed.

kou · 2025-10-10T00:36:36Z

cpp/src/arrow/flight/sql/odbc/tests/install_odbc.cmd

Do we need _amd64 in file name?

It seems that this doesn't have any amd64 specific part.

Yea that's fair, I have renamed to install_odbc

cpp/src/arrow/flight/sql/odbc/odbc_impl/system_trust_store.h

cpp/src/arrow/flight/sql/odbc/CMakeLists.txt

.github/workflows/cpp.yml

kou · 2025-10-10T00:46:49Z

.github/workflows/cpp.yml

+      - name: Register Flight SQL ODBC Driver
+        shell: cmd
+        run: |
+          call "cpp\src\arrow\flight\sql\odbc\install\install_amd64.cmd" ${{github.workspace}}\build\cpp\%ARROW_BUILD_TYPE%\libarrow_flight_sql_odbc.dll


cpp\src\arrow\flight\sql\odbc\script\ not ...\install\ may be better.
We will add uninstall scripts later, right?

Sure, I have renamed to script folder.

I am not sure if uninstall scripts are needed in this scenario and I am open to discussion. This script is for testing only. ODBC users will be able to install and uninstall the ODBC driver using a msi executable, so they won't need to use the script to install or uninstall the driver. Developers can also manually delete the driver registry if needed. This script can also be run a second time to change the registry to point to a different ODBC dll location, so the driver registry usually doesn't need to be deleted

OK. Then how about using test\ directory?

Or ci\scripts\?

I have renamed to tests. I think tests directory would work better in this case compared to ci\scripts since this is the only script for testing.

raulcd · 2025-10-10T08:16:21Z

cpp/src/arrow/flight/sql/odbc/odbc_impl/address_info.h

 #include "arrow/flight/sql/odbc/odbc_impl/platform.h"

 #include <sys/types.h>
+#include <cstdint>


was this missing previously?

Yes, I think so. I recall it was causing build issues on workflows. The build works with or without this header on my local machine (Windows MSVC), but the CI builds can have problems without the header

alinaliBQ · 2025-10-15T23:56:12Z

I think the failed Python / AMD64 Windows 2022 Python 3.13 (pull_request) check is not due to the changes in this PR

lidavidm · 2025-10-16T00:34:53Z

(FWIW, Kou/Raul are probably busy getting the new release out)

raulcd

it seems you have pushed a change to the testing submodule reverting it back to the previous commit, can you update the submodule to point again to the latest commit?
Current commit should be:
apache/arrow-testing@9a02925

raulcd · 2025-10-17T09:15:20Z

Thanks a lot for the PR, CI seems to be fixed now :)

alinaliBQ · 2025-10-17T18:56:44Z

it seems you have pushed a change to the testing submodule reverting it back to the previous commit, can you update the submodule to point again to the latest commit? Current commit should be: apache/arrow-testing@9a02925

Good catch, I have fixed it and reverted the testing submodule change. Thanks

kou

+1

…on (#47759) ### Rationale for this change ODBC driver needs to handle environment handle and connection handle allocation and deallocation. ### What changes are included in this PR? - Implement SQLAllocHandle and SQLFreeHandle APIs for allocating and deallocating environment and connection handles. - Tests ### Are these changes tested? - Tested locally on Windows MSVC. - Will be tested in CI after #47689 is merged ### Are there any user-facing changes? No * GitHub Issue: #46096 * GitHub Issue: #46097 Authored-by: Alina (Xi) Li <[email protected]> Signed-off-by: David Li <[email protected]>

…#47760) ### Rationale for this change ODBC driver needs to set and get environment attributes, such as ODBC driver version. ### What changes are included in this PR? - Implement SQLGetEnvAttr and SQLSetEnvAttr APIs for retrieving and setting environment attribute values - Tests ### Are these changes tested? Tested locally on Windows MSVC. Will be tested in CI after #47689 is merged ### Are there any user-facing changes? No * GitHub Issue: #46098 Lead-authored-by: Alina (Xi) Li <[email protected]> Co-authored-by: rscales <[email protected]> Signed-off-by: David Li <[email protected]>

Enable ODBC build in Windows MSVC workflow Fix build issue Register ODBC driver ODBC driver registration is required for ODBC tests (enabled in a separate PR) Address Sutou's comments

alinaliBQ · 2025-10-28T22:01:18Z

@kou Please let me know if you have any other comments, thanks! I would like to get this PR merged if possible

kou

+1

conbench-apache-arrow · 2025-10-29T18:51:10Z

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 6a08785.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 9 possible false positives for unstable benchmarks that are known to sometimes produce them.

github-actions bot added Component: C++ Component: FlightRPC awaiting review Awaiting review labels Oct 2, 2025

alinaliBQ commented Oct 2, 2025

View reviewed changes

github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Oct 2, 2025

kou changed the title ~~GH-46004: [C++][FlightRPC] Enable ODBC Build In C++ and Glib Workflows~~ GH-46004: [C++][FlightRPC] Enable ODBC Build In C++ and GLib Workflows Oct 3, 2025

alinaliBQ commented Oct 8, 2025

View reviewed changes

alinaliBQ force-pushed the gh-46004-build-odbc branch 2 times, most recently from c1d79fa to bb49757 Compare October 8, 2025 16:58

alinaliBQ changed the title ~~GH-46004: [C++][FlightRPC] Enable ODBC Build In C++ and GLib Workflows~~ GH-46004: [C++][FlightRPC] Enable ODBC Build In C++ Workflows Oct 8, 2025

alinaliBQ force-pushed the gh-46004-build-odbc branch 5 times, most recently from ec39f37 to 4d254df Compare October 9, 2025 19:49

alinaliBQ marked this pull request as ready for review October 9, 2025 20:22

alinaliBQ requested review from assignUser, jonkeane, kou, lidavidm and raulcd as code owners October 9, 2025 20:22

kou reviewed Oct 10, 2025

View reviewed changes

github-actions bot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Oct 10, 2025

raulcd reviewed Oct 10, 2025

View reviewed changes

github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes awaiting change review Awaiting change review labels Oct 10, 2025

alinaliBQ force-pushed the gh-46004-build-odbc branch from 032cb79 to c880d47 Compare October 16, 2025 18:19

raulcd reviewed Oct 17, 2025

View reviewed changes

github-actions bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Oct 17, 2025

alinaliBQ force-pushed the gh-46004-build-odbc branch from c880d47 to 8d79a82 Compare October 17, 2025 18:55

github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Oct 17, 2025

kou approved these changes Oct 17, 2025

View reviewed changes

github-actions bot added awaiting merge Awaiting merge and removed awaiting change review Awaiting change review labels Oct 17, 2025

alinaliBQ requested a review from raulcd October 20, 2025 17:36

alinaliBQ mentioned this pull request Oct 22, 2025

GH-46096: [C++][FlightRPC] Environment and Connection Handle Allocation #47759

Merged

alinaliBQ force-pushed the gh-46004-build-odbc branch from 5dce6e2 to e39d05b Compare October 22, 2025 23:12

alinaliBQ force-pushed the gh-46004-build-odbc branch from e39d05b to 4788fd9 Compare October 23, 2025 19:02

alinaliBQ mentioned this pull request Oct 23, 2025

GH-46098 : [C++][FlightRPC] ODBC Environment Attribute Implementation #47760

Merged

Build ODBC in C++ Windows Workflow

b304699

Enable ODBC build in Windows MSVC workflow Fix build issue Register ODBC driver ODBC driver registration is required for ODBC tests (enabled in a separate PR) Address Sutou's comments

alinaliBQ force-pushed the gh-46004-build-odbc branch from 4788fd9 to b304699 Compare October 27, 2025 23:35

kou approved these changes Oct 29, 2025

View reviewed changes

kou merged commit 6a08785 into apache:main Oct 29, 2025
44 of 46 checks passed

kou removed the awaiting merge Awaiting merge label Oct 29, 2025

kou mentioned this pull request Oct 29, 2025

[C++] Add ODBC build option to arrow root cmake file #46004

Closed

github-actions bot added the awaiting merge Awaiting merge label Oct 29, 2025

	# ifdef NDEBUG
	# define ARROW_DFATAL ::arrow::util::ArrowLogLevel::ARROW_WARNING

	// CAUTION: DCHECK_OK() always evaluates its argument, but other DCHECK*() macros
	// only do so in debug mode.

	# define ARROW_DCHECK(condition) \
	while (false) ARROW_IGNORE_EXPR(condition); \
	while (false) ::arrow::util::detail::NullLog()
	# define ARROW_DCHECK_OK(s) \
	ARROW_IGNORE_EXPR(s); \
	while (false) ::arrow::util::detail::NullLog()
	# define ARROW_DCHECK_EQ(val1, val2) \
	while (false) ARROW_IGNORE_EXPR(val1); \
	while (false) ARROW_IGNORE_EXPR(val2); \
	while (false) ::arrow::util::detail::NullLog()
	# define ARROW_DCHECK_NE(val1, val2) \
	while (false) ARROW_IGNORE_EXPR(val1); \
	while (false) ARROW_IGNORE_EXPR(val2); \
	while (false) ::arrow::util::detail::NullLog()
	# define ARROW_DCHECK_LE(val1, val2) \
	while (false) ARROW_IGNORE_EXPR(val1); \
	while (false) ARROW_IGNORE_EXPR(val2); \
	while (false) ::arrow::util::detail::NullLog()
	# define ARROW_DCHECK_LT(val1, val2) \
	while (false) ARROW_IGNORE_EXPR(val1); \
	while (false) ARROW_IGNORE_EXPR(val2); \
	while (false) ::arrow::util::detail::NullLog()
	# define ARROW_DCHECK_GE(val1, val2) \
	while (false) ARROW_IGNORE_EXPR(val1); \
	while (false) ARROW_IGNORE_EXPR(val2); \
	while (false) ::arrow::util::detail::NullLog()
	# define ARROW_DCHECK_GT(val1, val2) \
	while (false) ARROW_IGNORE_EXPR(val1); \
	while (false) ARROW_IGNORE_EXPR(val2); \
	while (false) ::arrow::util::detail::NullLog()

GH-46004: [C++][FlightRPC] Enable ODBC Build In C++ Workflows #47689

GH-46004: [C++][FlightRPC] Enable ODBC Build In C++ Workflows #47689

Uh oh!

Conversation

alinaliBQ commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alinaliBQ Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alinaliBQ commented Oct 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alinaliBQ commented Oct 15, 2025

Uh oh!

lidavidm commented Oct 16, 2025

Uh oh!

raulcd left a comment

Choose a reason for hiding this comment

Uh oh!

raulcd commented Oct 17, 2025

Uh oh!

alinaliBQ commented Oct 17, 2025

Uh oh!

kou left a comment

Choose a reason for hiding this comment

Uh oh!

alinaliBQ commented Oct 28, 2025

Uh oh!

kou left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

conbench-apache-arrow bot commented Oct 29, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

alinaliBQ commented Oct 2, 2025 •

edited

Loading

alinaliBQ Oct 8, 2025 •

edited

Loading